Skip to content

Example using async/http/internet/instance #72

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 24, 2021

Conversation

trevorturk
Copy link
Contributor

Description

  • Documentation re: Consider exposing Async::HTTP::Internet.instance. #69
  • Note I just added a standalone example without changing the others. I think this is appropriate since this isn't necessarily recommended for all use cases and requires an additional gem.
  • Note I didn't mention that you don't need to call internet&.close in an ensure block, since I think that's implied, but perhaps it's worth mentioning.
  • Apologies re: the whitespace changes. Please let me know if you'd like me to resubmit with a smaller diff.

Types of Changes

  • Documentation

Testing

  • N/A

@ioquatix
Copy link
Member

Thanks, this looks good to me, please submit a smaller diff :p

@trevorturk
Copy link
Contributor Author

Diff reduced! (But you should totally consider removing trailing whitespace some time in the future 😬 !)

@ioquatix
Copy link
Member

ioquatix commented Apr 24, 2021

I like to have my leading white space at the same indentation level as the rest of the code, it just looks tidier to me. Otherwise you need an editor capable of recognising the indentation level and automatically inserting it. Trailing whitespace on the other hand normally should be removed, except when it's sometime structural (e.g. in the license text).

@ioquatix ioquatix merged commit a69354e into socketry:master Apr 24, 2021
@trevorturk trevorturk deleted the thread-local-instance branch April 24, 2021 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants